🦋 Changeset detected

Latest commit: 605ea26

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
agents-docs	Ready	Preview	Comment	Oct 31, 2025 9:28pm
agents-manage-api	Ready	Preview	Comment	Oct 31, 2025 9:28pm
agents-manage-ui	Ready	Preview	Comment	Oct 31, 2025 9:28pm
agents-run-api	Ready	Preview	Comment	Oct 31, 2025 9:28pm

Claude finished @shagun-singh-inkeep's task —— View job

PR Review: Adding ID for MCP Error Context

Review Tasks

• Analyze the specific change in Agent.ts
• Research existing MCP error handling patterns
• Evaluate system design implications
• Provide comprehensive feedback

📋 Summary

This PR adds a single field fromSubAgent: this.config.id to the error event context when MCP tools fail. The change enhances error traceability by identifying which specific sub-agent encountered the MCP tool error.

🔍 Analysis

The Change:

// Added to line 572 in Agent.ts:572
context: {
  toolName,
  toolCallId,
  errorMessage,
+ fromSubAgent: this.config.id,  // NEW: Added for context
},

Location: agents-run-api/src/agents/Agent.ts:572 in the MCP tool error handling block

✅ Strengths

1. Consistency with Existing Patterns: The field name fromSubAgent follows the established convention used throughout the codebase for agent identification (found in 50+ locations including relation tools, conversations, and other error contexts).

2. Improved Error Debugging: In multi-agent systems, knowing which specific sub-agent encountered an MCP tool failure is valuable for debugging and troubleshooting.

3. Minimal Surface Area: Single field addition with no breaking changes to existing interfaces.

🚨 Areas for Consideration

High Priority

1. Inconsistent Field Usage in Same Method ⚠️
   Issue: The same method has another error recording at line ~802 that does NOT include fromSubAgent in the context:

   agentSessionManager.recordEvent(streamRequestId, 'error', this.config.id, {
     // ... no fromSubAgent field here
     context: {
       toolName: tool.name,
       serverUrl: tool.config.type === 'mcp' ? tool.config.mcp.server.url : 'unknown',
       operation: 'mcp_tool_discovery',
     },
   });

   Recommendation: For consistency, consider adding fromSubAgent: this.config.id to this error context as well, or document why this distinction exists.

2. Missing Type Safety Enhancement
   Issue: The ErrorEventData interface defines context?: any, but we're establishing a pattern where MCP errors include fromSubAgent.
   Recommendation: Consider strengthening the type system:

   interface MCPErrorContext {
     toolName: string;
     toolCallId: string;  
     errorMessage: string;
     fromSubAgent: string;
     // ... other MCP-specific fields
   }

Medium Priority

3. Field Naming Alignment
   Question: Other parts of the system use fromSubAgentId (e.g., in conversation messages, relation tools). Should this be fromSubAgentId instead of fromSubAgent for consistency?
   Evidence: Found 30+ usages of fromSubAgentId vs this single usage of fromSubAgent in error context.

4. Error Event Context Documentation
   Issue: The change adds domain-specific context without broader documentation of what fields should be present in different error scenarios.
   Suggestion: Consider adding JSDoc comments explaining the expected context structure for MCP errors.

💡 Suggested Improvements

1. Complete the Pattern: Apply this same enhancement to the other MCP error in the same file
2. Consider Standardization: Decide between fromSubAgent vs fromSubAgentId naming convention
3. Type Strengthening: Consider adding more specific typing for error contexts
4. Testing: Add test coverage for the error context structure to prevent regression

🎯 Verdict

Approved with Minor Suggestions - This is a solid defensive enhancement that improves debugging capabilities. The change is minimal, follows existing patterns, and adds valuable context for multi-agent error tracking. The suggestions above would make it even better but don't block the PR.

The inconsistency with the other error context in the same method should be addressed for completeness, but the core change is sound and valuable for operations and debugging.